Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Typesync #312

Merged

Conversation

lishaduck
Copy link
Contributor

Closes #295, closes #297, closes #193.

Copy link

changeset-bot bot commented Dec 3, 2024

⚠️ No Changeset found

Latest commit: 391a110

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Dec 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sheriff ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 5, 2025 1:56am

@lishaduck
Copy link
Contributor Author

lishaduck commented Dec 3, 2024

Works (correctly fails on Windows)!

@AndreaPontrandolfo
Copy link
Owner

AndreaPontrandolfo commented Dec 3, 2024

Yo @lishaduck, thank you as always!

Im having some real-life trouble right now, so i will probably be unable to review PRs this week And even the next.

@lishaduck lishaduck mentioned this pull request Dec 31, 2024
@lishaduck lishaduck changed the title Typesync update Update Typesync Dec 31, 2024
turbo.json Outdated
@@ -37,7 +37,9 @@
},
"//#manypkg": {},
"//#knip": {},
"typesync": {},
"//#typesync": {
"inputs": ["**/package.json", ".typesyncrc"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm, either we don't specify the inputs or we add an empty .typesyncrc file. Otherwise the first time that we add a config file for typescync we are gonna run into trouble.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I'd added a .typesyncrc file, but even if not, is there an issue with specifying a non-existent file?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an issue with specifying a non-existent file?

Definitely yes. It creates confusion both now and for our future selfs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still waiting on this

Copy link
Owner

@AndreaPontrandolfo AndreaPontrandolfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lishaduck

Please remove the whole tilde (~) thing. It's not needed and it makes my head spin.

turbo.json Outdated
@@ -37,7 +37,9 @@
},
"//#manypkg": {},
"//#knip": {},
"typesync": {},
"//#typesync": {
"inputs": ["**/package.json", ".typesyncrc"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an issue with specifying a non-existent file?

Definitely yes. It creates confusion both now and for our future selfs.

@lishaduck
Copy link
Contributor Author

Please remove the whole tilde (~) thing. It's not needed and it makes my head spin.

I can, but first, can you read the rational?

Pedantically, the most correct thing would be to pin types to =, but that's a bit over top. ~ balances it.

@AndreaPontrandolfo
Copy link
Owner

Please remove the whole tilde (~) thing. It's not needed and it makes my head spin.

I can, but first, can you read the rational?

I did. Still don't get it.

As an example, think about Lodash. Lodash stopped being updated 9 years ago. What does it matter which version of the typings we are importing? Of course we want the latest.

Seriously i don't want to deal with any of that.

@lishaduck
Copy link
Contributor Author

I did. Still don't get it.

As an example, think about Lodash. Lodash stopped being updated 9 years ago. What does it matter which version of the typings we are importing? Of course we want the latest.

Lodash v4 is still "in progress", but let's take react:
Say you're a package with a minimum dependency on React "^18.1", and React 18.2.0 comes out. You could upgrade the types package to 18.2.0, but then you wouldn't get any warning when you use 18.2.0 features, and could inadvertently make a breaking change. With a lockfile, you have to manually upgrade either way, so it doesn't really make a difference if you want the latest, you'll get an older version until you upgrade either way.

Seriously i don't want to deal with any of that.

I'll go change it, I just wanted to ensure you understood the why.

turbo.json Outdated
@@ -37,7 +37,9 @@
},
"//#manypkg": {},
"//#knip": {},
"typesync": {},
"//#typesync": {
"inputs": ["**/package.json", ".typesyncrc"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still waiting on this

@AndreaPontrandolfo
Copy link
Owner

@lishaduck on a side note, does pnpm dev works on your end? It's still broken on mine.

@lishaduck
Copy link
Contributor Author

@lishaduck on a side note, does pnpm dev works on your end? It's still broken on mine.

I'd gotten it working, but it seems to have broken again.

@lishaduck
Copy link
Contributor Author

lishaduck commented Jan 2, 2025

@lishaduck on a side note, does pnpm dev works on your end? It's still broken on mine.

I'd gotten it working, but it seems to have broken again.

Made some tweaks. It's easy enough to get to work, but it's a little finicky. Are you fine with removing dev:norebuild (the issues are just that I didn't want to break it)?

@lishaduck
Copy link
Contributor Author

Are you fine with removing dev:norebuild (the issues are just that I didn't want to break it)?

I think I figured out how to fix it.

@lishaduck
Copy link
Contributor Author

lishaduck commented Jan 3, 2025

@AndreaPontrandolfo, ping

(I'd like to get all my open PRs here and in fsecond merged today, if possible, as school restarts on Monday.)

turbo.json Outdated Show resolved Hide resolved
Not quite optimal, and requires a bit of bootstrapping, but it works alright.
@AndreaPontrandolfo AndreaPontrandolfo merged commit 02da6c9 into AndreaPontrandolfo:master Jan 5, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Typesync Monorepo support
2 participants